Skip to content

Conversation

folkertdev
Copy link
Contributor

@folkertdev folkertdev commented Mar 29, 2025

Use some # directives to make sure the code checks on x86_64, and does not produce errors on other platforms. This example still used an older version of #[naked], and because the snippet was ignored that was missed before.

I'm not sure when this gets built on CI exactly, so it might be worthwhile to try and build it for a non-x86_64 architecture to make sure that works. I'm not sure how to verify locally that e.g. on aarch64 this code works without errors/warnings.

try-job: aarch64-apple
try-job: x86_64-msvc-2

@rustbot
Copy link
Collaborator

rustbot commented Mar 29, 2025

r? @GuillaumeGomez

rustbot has assigned @GuillaumeGomez.
They will have a look at your PR within the next two weeks and either review your PR or reassign to another reviewer.

Use r? to explicitly pick a reviewer

@rustbot rustbot added PG-exploit-mitigations Project group: Exploit mitigations S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Mar 29, 2025
@rustbot
Copy link
Collaborator

rustbot commented Mar 29, 2025

Some changes occurred in src/doc/unstable-book/src/compiler-flags/sanitizer.md

cc @rust-lang/project-exploit-mitigations, @rcvalle

@GuillaumeGomez
Copy link
Member

Nice trick, thanks!

@bors r+ rollup

@bors
Copy link
Collaborator

bors commented Mar 30, 2025

📌 Commit b00b6a8 has been approved by GuillaumeGomez

It is now in the queue for this repository.

@bors bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Mar 30, 2025
jhpratt added a commit to jhpratt/rust that referenced this pull request Mar 30, 2025
…-check-block, r=GuillaumeGomez

unstable book: in a sanitizer example, check the code

Use some `#` directives to make sure the code checks on x86_64, and does not produce errors on other platforms. This example still used an older version of `#[naked]`, and because the snippet was ignored that was missed before.

I'm not sure when this gets built on CI exactly, so it might be worthwhile to try and build it for a non-x86_64 architecture to make sure that works. I'm not sure how to verify locally that e.g. on aarch64 this code works without errors/warnings.
bors added a commit to rust-lang-ci/rust that referenced this pull request Mar 30, 2025
Rollup of 6 pull requests

Successful merges:

 - rust-lang#139044 (bootstrap: Avoid cloning `change-id` list)
 - rust-lang#139111 (Properly document FakeReads)
 - rust-lang#139113 (unstable book: in a sanitizer example, check the code)
 - rust-lang#139122 (Remove attribute `#[rustc_error]`)
 - rust-lang#139132 (Improve hir_pretty for struct expressions.)
 - rust-lang#139141 (Switch some rustc_on_unimplemented uses to diagnostic::on_unimplemented)

r? `@ghost`
`@rustbot` modify labels: rollup
@jhpratt
Copy link
Member

jhpratt commented Mar 30, 2025

@bors r-

#139147 (comment)

@bors bors added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. labels Mar 30, 2025
@folkertdev folkertdev force-pushed the sanitizer-unstable-book-check-block branch 2 times, most recently from acb3898 to f8bc9ac Compare April 1, 2025 08:45
@folkertdev
Copy link
Contributor Author

platforms are hard...

This now runs the example just on x86_64: it already used x86_64 inline assembly, so the code was kind of meaningless on any other platform. The goal of the unstable book is not really to test the behavior, but more that the example compiles and is up-to-date, so this seemed like the best option to me.

@rustbot ready

I also added a try build to make sure this actually works on non-x86_64.

@rustbot rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Apr 1, 2025
@folkertdev folkertdev force-pushed the sanitizer-unstable-book-check-block branch from f8bc9ac to 0ffa4c6 Compare April 1, 2025 08:59
@rust-log-analyzer

This comment has been minimized.

@folkertdev folkertdev force-pushed the sanitizer-unstable-book-check-block branch from 0ffa4c6 to 5aec89a Compare April 1, 2025 17:39
@rust-log-analyzer

This comment has been minimized.

@folkertdev folkertdev force-pushed the sanitizer-unstable-book-check-block branch from 5aec89a to 1c5c24f Compare April 1, 2025 19:16
@bors
Copy link
Collaborator

bors commented Apr 20, 2025

☔ The latest upstream changes (presumably #140043) made this pull request unmergeable. Please resolve the merge conflicts.

@apiraino
Copy link
Contributor

@folkertdev is this patch only waiting on a rebase? thanks!

@rustbot author

@rustbot rustbot added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels May 22, 2025
rust-bors bot added a commit that referenced this pull request Aug 31, 2025
…k, r=<try>

unstable book: in a sanitizer example, check the code

try-job: aarch64-apple
@rust-bors
Copy link

rust-bors bot commented Aug 31, 2025

☀️ Try build successful (CI)
Build commit: c651b66 (c651b66aac3166492ef2032184c18ba6c3d572a0, parent: 1bc901e0cab0f150e3bcdb9e7cf99ab085682b3e)

@folkertdev
Copy link
Contributor Author

Yes, these examples were not meant to be run (on CI or tests). E.g., The examples with inline assembly are meant to be run on AMD64 or equivalent, and all the examples with CFI enabled are meant to fail with Illegal Instruction or equivalent, but if you're able to make them run within those constraints only that should be fine I guess.

I found this because the usage of naked_asm! was outdated in the example. By running the code it also gets checked.

The current implementation passes the try-job, so I think we're good here.

@rcvalle
Copy link
Member

rcvalle commented Sep 2, 2025

SG! Let's give it a try.

@rcvalle
Copy link
Member

rcvalle commented Sep 2, 2025

@bors r+

@bors
Copy link
Collaborator

bors commented Sep 2, 2025

📌 Commit 4524d9d has been approved by rcvalle

It is now in the queue for this repository.

@bors bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Sep 2, 2025
GuillaumeGomez added a commit to GuillaumeGomez/rust that referenced this pull request Sep 2, 2025
…-check-block, r=rcvalle

unstable book: in a sanitizer example, check the code

Use some `#` directives to make sure the code checks on x86_64, and does not produce errors on other platforms. This example still used an older version of `#[naked]`, and because the snippet was ignored that was missed before.

I'm not sure when this gets built on CI exactly, so it might be worthwhile to try and build it for a non-x86_64 architecture to make sure that works. I'm not sure how to verify locally that e.g. on aarch64 this code works without errors/warnings.

try-job: aarch64-apple
GuillaumeGomez added a commit to GuillaumeGomez/rust that referenced this pull request Sep 2, 2025
…-check-block, r=rcvalle

unstable book: in a sanitizer example, check the code

Use some `#` directives to make sure the code checks on x86_64, and does not produce errors on other platforms. This example still used an older version of `#[naked]`, and because the snippet was ignored that was missed before.

I'm not sure when this gets built on CI exactly, so it might be worthwhile to try and build it for a non-x86_64 architecture to make sure that works. I'm not sure how to verify locally that e.g. on aarch64 this code works without errors/warnings.

try-job: aarch64-apple
bors added a commit that referenced this pull request Sep 2, 2025
Rollup of 8 pull requests

Successful merges:

 - #139113 (unstable book: in a sanitizer example, check the code)
 - #145823 (editorconfig: don't use nonexistent syntax)
 - #145962 (Ensure we emit an allocator shim when only some crate types need one)
 - #146032 (Explicity disable LSX feature for `loongarch64-unknown-none` target)
 - #146090 (Derive `PartialEq` for `InvisibleOrigin`)
 - #146120 (Correct typo in `rustc_errors` comment)
 - #146121 (fix: Filter suggestion parts that match existing code)
 - #146134 (llvm: nvptx: Layout update to match LLVM)

r? `@ghost`
`@rustbot` modify labels: rollup
tgross35 added a commit to tgross35/rust that referenced this pull request Sep 3, 2025
…-check-block, r=rcvalle

unstable book: in a sanitizer example, check the code

Use some `#` directives to make sure the code checks on x86_64, and does not produce errors on other platforms. This example still used an older version of `#[naked]`, and because the snippet was ignored that was missed before.

I'm not sure when this gets built on CI exactly, so it might be worthwhile to try and build it for a non-x86_64 architecture to make sure that works. I'm not sure how to verify locally that e.g. on aarch64 this code works without errors/warnings.

try-job: aarch64-apple
bors added a commit that referenced this pull request Sep 3, 2025
Rollup of 8 pull requests

Successful merges:

 - #139113 (unstable book: in a sanitizer example, check the code)
 - #145279 (Constify conversion traits (part 1))
 - #145414 (unicode-table-generator refactors)
 - #145823 (editorconfig: don't use nonexistent syntax)
 - #145944 (std: Start supporting WASIp2 natively )
 - #145961 (resolve: Avoid a regression from splitting prelude into two scopes)
 - #146032 (Explicity disable LSX feature for `loongarch64-unknown-none` target)
 - #146106 (fix(lexer): Only allow horizontal whitespace in frontmatter )

r? `@ghost`
`@rustbot` modify labels: rollup
bors added a commit that referenced this pull request Sep 3, 2025
Rollup of 8 pull requests

Successful merges:

 - #139113 (unstable book: in a sanitizer example, check the code)
 - #145279 (Constify conversion traits (part 1))
 - #145414 (unicode-table-generator refactors)
 - #145823 (editorconfig: don't use nonexistent syntax)
 - #145944 (std: Start supporting WASIp2 natively )
 - #145961 (resolve: Avoid a regression from splitting prelude into two scopes)
 - #146032 (Explicity disable LSX feature for `loongarch64-unknown-none` target)
 - #146106 (fix(lexer): Only allow horizontal whitespace in frontmatter )

r? `@ghost`
`@rustbot` modify labels: rollup
@Zalathar
Copy link
Contributor

Zalathar commented Sep 3, 2025

Failed in rollup, in x86_64-msvc-2: #146141 (comment)

@bors r-

@bors bors added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. labels Sep 3, 2025
this uses some # directives to make sure the code works on x86_64, and does not produce errors on other platforms
@folkertdev folkertdev force-pushed the sanitizer-unstable-book-check-block branch from 4524d9d to b301e3a Compare September 3, 2025 11:12
@rustbot
Copy link
Collaborator

rustbot commented Sep 3, 2025

This PR was rebased onto a different master commit. Here's a range-diff highlighting what actually changed.

Rebasing is a normal part of keeping PRs up to date, so no action is needed—this note is just to help reviewers.

@folkertdev
Copy link
Contributor Author

hmm yes, we should be careful with inline assembly: I've now made it an extern "sysv64", which is what it is implicitly because of the inline assembly. I've also added target_os = linux, because the goal is just to check that this builds.

@folkertdev
Copy link
Contributor Author

the joys of assembly and platform-specific code...

@bors2 try

@rust-bors

This comment has been minimized.

rust-bors bot added a commit that referenced this pull request Sep 3, 2025
…k, r=<try>

unstable book: in a sanitizer example, check the code

try-job: aarch64-apple
try-job: x86_64-msvc-2
@rust-bors
Copy link

rust-bors bot commented Sep 3, 2025

☀️ Try build successful (CI)
Build commit: 2ebfe3a (2ebfe3aaf646bf8f2684c55ea56971df6178359f, parent: 51ff895062ba60a7cba53f57af928c3fb7b0f2f4)

@folkertdev
Copy link
Contributor Author

@rustbot ready

@rustbot rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Sep 4, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
PG-exploit-mitigations Project group: Exploit mitigations S-waiting-on-review Status: Awaiting review from the assignee but also interested parties.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

9 participants